Skip to content

perf(particlesys): Optimize angleBetween() in Particle System #2218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
xezon wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(particlesys): Optimize angleBetween() in Particle System #2218

xezon wants to merge 1 commit into from

Conversation

@xezon
Copy link

@xezon xezon commented Jan 29, 2026

This change optimizes the angleBetween() function a bit by removing 2 redundant length function calls, which each contain

  • 1 square root
  • 2 multiplications
  • 1 addition

It is used by the Particle System update.

Not measured, because impact is expected very minor.

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Gen Relates to Generals ZH Relates to Zero Hour labels Jan 29, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

Optimized the angleBetween() function by caching vector length calculations, eliminating 2 redundant calls to Coord2D::length(). Each eliminated call saves 1 sqrt operation, 2 multiplications, and 1 addition.

Key changes:

  • Computed lengthA and lengthB once upfront instead of calling length() 4 times total
  • Removed redundant null pointer checks (the function is file-static and only called with stack-allocated objects)
  • Fixed typo: "dotproduct" → "dot product"
  • Changed return literal from 0.0 to 0.0f for consistency

The optimization is mathematically correct and maintains the same behavior for all valid inputs. The null pointer check removal is safe given the single call site at ParticleSys.cpp:415 passes stack-allocated addresses.

Confidence Score: 5/5
  • This PR is safe to merge with no risk - it's a straightforward performance optimization
  • The change is a simple, mathematically sound optimization that reduces redundant calculations without altering correctness. The removal of null checks is safe given the function's file-local scope and single call site that never passes nulls
  • No files require special attention

Important Files Changed
Filename Overview
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp Optimized angleBetween() by caching length calculations, eliminating 2 redundant sqrt operations per call

Sequence Diagram

 
sequenceDiagram
    participant Particle as Particle::updateParticle()
    participant angleBetween as angleBetween()
    participant Coord2D as Coord2D::length()
    
    Note over Particle: m_particleUpTowardsEmitter == true
    Particle->>Particle: Create upVec {0, 1}
    Particle->>Particle: Create emitterDir from positions
    Particle->>angleBetween: angleBetween(&upVec, &emitterDir)
    
    Note over angleBetween: Before optimization:<br/>4 calls to length()
    Note over angleBetween: After optimization:<br/>2 calls to length()
    
    angleBetween->>Coord2D: vecA->length()
    Coord2D-->>angleBetween: lengthA
    angleBetween->>Coord2D: vecB->length()
    Coord2D-->>angleBetween: lengthB
    
    Note over angleBetween: Check if both lengths != 0
    
    alt Both lengths are zero
        angleBetween-->>Particle: return 0.0f
    else Valid vectors
        Note over angleBetween: Calculate dot product<br/>Calculate cosTheta<br/>Calculate theta using ACos
        angleBetween-->>Particle: return ±theta
    end
    
    Particle->>Particle: Set m_angleZ = theta + PI
Loading

Mauller
Mauller approved these changes Jan 30, 2026
View reviewed changes
Copy link

@Mauller Mauller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zoom zoom

Mauller
Mauller reviewed Jan 30, 2026
View reviewed changes
Core/GameEngine/Source/GameClient/System/ParticleSys.cpp
const Real lengthA = vecA->length();
const Real lengthB = vecB->length();

if (!(lengthA && lengthB)) {
Copy link

@Mauller Mauller Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be !(vecA || vecB) before their use, or can they not be null at any point?

Copy link
Author

@xezon xezon Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I looked at call sites and they were not null, so I omitted null tests. I can relook.

Copy link
Author

@xezon xezon Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is only one caller and it never passes null, so this is safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mauller Mauller Mauller approved these changes

Assignees

No one assigned

Labels

Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern ZH Relates to Zero Hour

Projects

None yet

Milestone

Runtime performance fixes & optimizat...

Development

Successfully merging this pull request may close these issues.

2 participants

@xezon @Mauller